HTTP/3 (QUIC) foundation: Phase 6 (Stream layer and flow control) - #27882
HTTP/3 (QUIC) foundation: Phase 6 (Stream layer and flow control)#27882quaesitor-scientiam wants to merge 4 commits into
Conversation
a261faf to
f78f16e
Compare
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Implements Phase 6 of the HTTP/3 (QUIC) foundation by adding QUIC stream-layer primitives (stream IDs + stream set + send/recv half state), stream reassembly with final-size validation, and flow-control windows, alongside extensive unit/integration tests and new stream-related frame parsing/encoding.
Changes:
- Added stream-related QUIC frames (STREAM/RESET_STREAM/STOP_SENDING plus flow-control frames) with parse/encode + regression tests.
- Introduced
QuicStream/QuicStreamSet(stream ID categorization, stream creation rules) and stream reassembly (StreamReassembler) with FINAL_SIZE enforcement. - Added flow control window types (
FlowControlWindow,ReceiveWindow) and tests, plus Phase 6 progress documentation.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vlib/net/quic/frame.v | Adds stream + flow-control frame types, parsing, and encoding helpers |
| vlib/net/quic/frame_test.v | Expands frame tests for STREAM and related frames; fixes sum-type narrowing test pattern |
| vlib/net/quic/initial_exchange_test.v | Fixes sum-type narrowing issue by binding before match |
| vlib/net/quic/stream.v | Introduces stream ID categorization, stream halves/state, and QuicStreamSet |
| vlib/net/quic/stream_test.v | Adds unit tests for stream IDs, stream creation, and send/recv half transitions |
| vlib/net/quic/stream_reassembly.v | Adds per-stream reassembly with overlap validation + FINAL_SIZE checks |
| vlib/net/quic/stream_reassembly_test.v | Adds reassembly tests including overlap and FINAL_SIZE_ERROR cases |
| vlib/net/quic/stream_layer_test.v | Adds multi-stream interleaved integration test with connection-level receive window |
| vlib/net/quic/flow_control.v | Adds connection/stream flow-control window bookkeeping + initial limit helpers |
| vlib/net/quic/flow_control_test.v | Adds unit tests for flow-control invariants and interplay scenarios |
| vlib/net/quic/PROGRESS.md | Marks Phase 6 as complete and documents key design decisions |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…LOCKED bound, reset-state clobbering All from Copilot, vlang#27882 pullrequestreview-4888843234. - stream_reassembly.v: max_stream_buffered_bytes capped the ABSOLUTE stream end offset, not buffered memory -- since `received` was never drained, this made any stream over 1 MiB permanently unreassemblable, even for entirely in-order data. Added base_offset + discard() so a caller (Phase 9's future application-read path) can free consumed bytes, sliding the buffered window forward; the cap now bounds buffered-but-not-yet-consumed data instead of total stream size. Also now bounds total PENDING (out-of-order) bytes, not just fragment count. - frame.v: encode_stream_frame's `offset + u64(data.len) > max_varint` check could itself overflow for a caller-supplied offset near u64::MAX, wrapping to a small value and silently passing. Unlike the parse-side check (both operands there come from decode_varint, inherently bounded to max_varint), offset here has no such bound. Rewritten as an overflow-safe check (bound offset alone first, then a subtraction-based comparison). - frame.v: STREAMS_BLOCKED carries the same `maximum_streams` semantic as MAX_STREAMS but was missing RFC 9000 §4.6's identical 2^60 cap (verified against the primary RFC text, §19.14) -- added to both parse and encode paths, matching the MAX_STREAMS fix from the previous review round. - stream.v: `note_size_known`/`note_data` could unconditionally transition state to data_recvd even after mark_reset_recvd already set reset_recvd -- RFC 9000 §3.2 makes Reset Recvd terminal, and a reordered STREAM frame arriving after RESET_STREAM raced ahead of it on the wire would silently clobber the reset classification. Both functions now no-op once state is reset_recvd/reset_read. 9 new regression tests. Full suite: 28/28. Co-Authored-By: WOZCODE <contact@withwoz.com>
Extends frame.v with all stream-related frames (STREAM, RESET_STREAM, STOP_SENDING, MAX_DATA, MAX_STREAM_DATA, MAX_STREAMS, DATA_BLOCKED, STREAM_DATA_BLOCKED, STREAMS_BLOCKED). Adds stream.v (stream ID categories, send/recv state machines, QuicStreamSet), stream_reassembly.v (per-stream offset-ordered reassembly with final-size reconciliation, mirroring Phase 4's crypto_stream.v), and flow_control.v (connection- and stream-level windows, the RFC 9000 §4.1 initial-limit naming inversion resolved in one place). /vreview found and fixed a real design footgun: QuicStream.send/recv were Optional value fields, so mutating the unwrapped copy via note_data()/note_size_known() silently didn't persist unless the caller remembered to reassign it back. Fixed by switching to nilable pointers (matching Tls13ClientHandshake.verified_chain's established convention) before any real caller could hit it. Also fixed two mechanical V-compiler quirks: match on a repeated array-index expression stops reliably narrowing a sum type past a certain variant count (affected two pre-existing tests too), and a stale "type 0x08 unimplemented" test now that Phase 6 implements STREAM frames there. Includes the plan's own named integration test: three interleaved streams (client bidi, server-initiated uni, second client bidi) with connection-level flow control tracked across all three. Co-Authored-By: WOZCODE <contact@withwoz.com>
…EAMS frames /vreview pass on this branch after rebasing onto master (Phase 5 merged): found and fixed 2 confirmed sibling-parity gaps before push. - frame.v: parse_stream_frame/encode_stream_frame were missing the "offset + length <= 2^62-1" bound RFC 9000 §19.8 requires (identical to §19.6's CRYPTO-frame requirement, which parse_crypto_frame/ encode_crypto_frame already correctly enforce in this same file). A STREAM frame with offset near 2^62-1 and nonzero data previously parsed successfully with no FRAME_ENCODING_ERROR, only accidentally caught downstream by stream_reassembly.v's unrelated 1 MiB DoS cap. - frame.v: parse_max_streams_frame/encode_max_streams_frame had no check against RFC 9000 §4.6's 2^60 cap (verified against the primary RFC text). transport_parameters.v already enforces this exact limit on the transport-parameter half of the same RFC requirement, with full boundary tests -- the frame half, added in this same PR, wasn't checked against that in-repo sibling before this pass. Also documented (not changed): stream.v's open_local_stream has no max_streams enforcement, unlike its receive-side sibling get_or_create -- plausibly a future Phase 9 caller's responsibility (it owns the peer's currently-advertised limit), but wasn't documented as deferred the way other genuinely-deferred hooks in this file are. Added that note. 5 new regression tests (STREAM offset+length bound: encode, parse with explicit length, parse with implicit/LEN-bit-clear length; MAX_STREAMS 2^60 bound: encode, parse, boundary-accepted). Full suite: 28/28. Co-Authored-By: WOZCODE <contact@withwoz.com>
…LOCKED bound, reset-state clobbering All from Copilot, vlang#27882 pullrequestreview-4888843234. - stream_reassembly.v: max_stream_buffered_bytes capped the ABSOLUTE stream end offset, not buffered memory -- since `received` was never drained, this made any stream over 1 MiB permanently unreassemblable, even for entirely in-order data. Added base_offset + discard() so a caller (Phase 9's future application-read path) can free consumed bytes, sliding the buffered window forward; the cap now bounds buffered-but-not-yet-consumed data instead of total stream size. Also now bounds total PENDING (out-of-order) bytes, not just fragment count. - frame.v: encode_stream_frame's `offset + u64(data.len) > max_varint` check could itself overflow for a caller-supplied offset near u64::MAX, wrapping to a small value and silently passing. Unlike the parse-side check (both operands there come from decode_varint, inherently bounded to max_varint), offset here has no such bound. Rewritten as an overflow-safe check (bound offset alone first, then a subtraction-based comparison). - frame.v: STREAMS_BLOCKED carries the same `maximum_streams` semantic as MAX_STREAMS but was missing RFC 9000 §4.6's identical 2^60 cap (verified against the primary RFC text, §19.14) -- added to both parse and encode paths, matching the MAX_STREAMS fix from the previous review round. - stream.v: `note_size_known`/`note_data` could unconditionally transition state to data_recvd even after mark_reset_recvd already set reset_recvd -- RFC 9000 §3.2 makes Reset Recvd terminal, and a reordered STREAM frame arriving after RESET_STREAM raced ahead of it on the wire would silently clobber the reset classification. Both functions now no-op once state is reset_recvd/reset_read. 9 new regression tests. Full suite: 28/28. Co-Authored-By: WOZCODE <contact@withwoz.com>
report-missing-fn-doc CI flagged advertised_limit/mark_advertised in flow_control.v (new since the master rebase picked up 3 more upstream CI fixes). While in these files, closed the remaining pre-existing gaps missdoc found across the whole stream-layer module (flow_control.v, stream.v, stream_reassembly.v) rather than leaving them for a future round. Doc-comment-only change; full net/quic suite still 28/28.
979e063 to
75da671
Compare
|
@JalonSolov none of the CI failures are related to this PR |
Is there an issue to merge this? |
|
copilot lies. :-( I would feel better with a clean final review. |
So another delay in finishing http/3 |
Summary
Phase 6 of HTTP/3/QUIC support (#27675): the stream layer and flow
control, continuing from the completed Phase 5 work in #27881 (full
handshake completion). Builds on the completed Phase 0-5 foundation —
#27680, #27877, #27880, #27881 — all merged, and targets
masterdirectly.See
vlib/net/quic/PROGRESS.mdfor the exact phase-by-phase checklist.
Scope
frame.vextended with every stream-related frame: STREAM, RESET_STREAM,STOP_SENDING, MAX_DATA, MAX_STREAM_DATA, MAX_STREAMS, DATA_BLOCKED,
STREAM_DATA_BLOCKED, STREAMS_BLOCKED.
stream.v—QuicStream, stream ID categories (RFC 9000 §2.1:client/server-initiated, bidirectional/unidirectional, encoded in the
low 2 bits), independent send/receive state machines per bidi stream.
Even this client-first phase must correctly receive server-initiated
unidirectional streams from day one -- HTTP/3's control stream and
QPACK encoder/decoder streams are all server-to-client uni streams,
not deferrable to a later server-support phase.
flow_control.v— connection- and stream-level windows enforcedtogether (a frame within its own stream's window can still be blocked
by the connection-level aggregate window and vice versa); three
separate stream-level initial limits depending on who opened the stream
and its directionality; auto-window-growth heuristic to avoid
throughput stalls; RESET_STREAM's Final Size reconciled against
previously-received offsets (FINAL_SIZE_ERROR on mismatch).
stream_reassembly.v— per-stream offset-ordered reassembly, mirroringPhase 4's crypto_stream.v pattern but for STREAM frames' implicit-length-
at-end-of-packet handling.
Test plan
(
id -> categoryandcategory -> first id).named test, confirming a server-initiated uni stream has exactly a
receive half and no send half from the client's perspective, plus
exercised end-to-end in the integration test below.
connection window blocking a send the stream window alone would
allow, and vice versa).
smaller than already-received data, a final size that later changes,
a final size conflicting with an already-buffered out-of-order
fragment, and data arriving after the final size that would exceed it.
bidi, server-initiated uni, second client bidi), STREAM frames
delivered genuinely interleaved and out of order, each independently
reassembled while one connection-level window tracks the running
total across all three.
./vnew test <path>-- 28/28files in
vlib/net/quic/../vnew fmt -wapplied to all touched.vfiles.upstream/master(2026-08-07) now that HTTP/3 (QUIC) foundation: Phase 5 (Full handshake completion) #27881merged (also via squash) -- clean
git rebase --onto, no testadaptation needed (unlike HTTP/3 (QUIC) foundation: Phase 5 (Full handshake completion) #27881's rebase, no upstream validation
change broke this branch's own tests).
/vreview(full A-G pass, run once while writing this phase and againafter the rebase before push) found and fixed:
Before commit:
QuicStream.send/recvwere Optional VALUE fields, somutating the unwrapped copy via
note_data()/note_size_known()lookedlike in-place mutation but silently didn't persist unless the caller
remembered to reassign it back (
s.recv = recv) -- a future caller couldeasily miss this and get stale
state/final_sizewhile the underlyingdata was still correct. Fixed by switching to nilable pointers (matching
Tls13ClientHandshake.verified_chain's established convention) before anyreal caller could hit it, eliminating the bug class rather than
documenting the trap. Also fixed two mechanical V-compiler quirks along
the way:
matchon a repeated array-index expression (frames[N]) stopsreliably narrowing a sum type once it has enough variants -- affected two
pre-existing tests in
frame_test.v/initial_exchange_test.vthat hadworked fine with fewer variants -- and a pre-existing "frame type 0x08 is
unimplemented" test that became false once this phase implemented STREAM
frames at that exact value (retargeted to 0x1e/HANDSHAKE_DONE).
After the rebase, before push: 2 confirmed sibling-parity gaps, both
citations verified against the primary RFC 9000 text before fixing.
parse_stream_frame/encode_stream_framewere missing RFC 9000 §19.8's"offset + length ≤ 2^62-1" bound -- identical to §19.6's CRYPTO-frame
requirement, which
parse_crypto_frame/encode_crypto_framealreadycorrectly enforce in this same file.
parse_max_streams_frame/encode_max_streams_framewere missing RFC 9000 §4.6's 2^60 cap --already correctly enforced (with its own boundary tests) on the
transport-parameter half of the identical requirement in
transport_parameters.v. Fixed both, 5 new regression tests. Alsodocumented (not changed, since it's plausibly a future Phase 9 caller's
responsibility):
open_local_streamhas nomax_streamsenforcement,unlike its receive-side sibling
get_or_create.🧙 Built with WOZCODE